Skip to content

Fix multiline function signature parsing #580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Jul 25, 2025

👨 @fredrikekre had an issue where the following parse was weird

julia> using JuliaSyntax

shell> cat bug.jl
function (
        ::A
        )()
end

julia> node = JuliaSyntax.parseall(JuliaSyntax.GreenNode, read("bug.jl", String))
     1:39     │[toplevel]
     1:38     │  [function]
     1:8      │    function
     9:9      │    Whitespace
    10:32     │    [tuple]
    10:10     │      (
    11:19     │      NewlineWs
    20:22     │      [::]
    20:21     │        ::
    22:22     │        Identifier       ✔
    23:31     │      NewlineWs
    32:32     │      )
    33:35     │    [block]
    33:34     │      [tuple]
    33:33     │        (
    34:34     │        )
    35:35     │      NewlineWs
    36:38     │    end
    39:39     │  NewlineWs

I let Claude lose on it and occording to it the issue was that with newlines the peek(ps, 2) check didn't function properly.

This is a bit AI slop so it might not make sense to merge but it might point to where the issue is at least.


🤖

Multiline function signatures with type annotations were incorrectly
parsed as tuples instead of calls when newlines appeared between
parentheses. For example:

function (
    ::A
)()
end

was parsed as (function (tuple ...) (block)) instead of the correct
(function (call (parens ...)) (block)), inconsistent with the
single-line version function (::A)() end.

The issue was in parse_function_signature where peek(ps, 2) was used
to detect if a call pattern follows the closing parenthesis, but this
didn't skip newlines.

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

Multiline function signatures with type annotations were incorrectly
parsed as tuples instead of calls when newlines appeared between
parentheses. For example:

```julia
function (
    ::A
)()
end
```

was parsed as `(function (tuple ...) (block))` instead of the correct
`(function (call (parens ...)) (block))`, inconsistent with the
single-line version `function (::A)() end`.

The issue was in parse_function_signature where `peek(ps, 2)` was used
to detect if a call pattern follows the closing parenthesis, but this
didn't skip newlines. Changed to `peek(ps, 2, skip_newlines=true)` to
properly detect the opening parenthesis of the argument list even when
separated by whitespace.

🤖 Generated with [Claude Code](https://claude.ai/code)
_needs_parse_call = peek(ps, 2) ∈ KSet"( ."
# Check if we should skip newlines - only for specific cases
# where we have a single type annotation like (::T)
_skip_newlines = !had_commas && num_subexprs == 1 &&
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be honest in that I don't really know what is going on here...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same condition as the maybe_grouping_parens below - should be refactored if that's intended.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the odd thing about this is that it needs to ignore whitespace before the first peek token, but not the second one. I don't know that these conditions here make any sense. We don't have a peek api like that, but I think that's what's needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the correct solution is to move the _needs_parse_call outside the do block - it seems weird that it would affect needs_parameters, because if there are parameters, the peek will never actually see the parenthesis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants